Skip to content

Ensure OOD app installs work for site image builds #843

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Ensure OOD app installs work for site image builds #843

wants to merge 13 commits into from
+26 −11

Conversation

@bertiethorpe
Copy link
Member

@bertiethorpe bertiethorpe commented Nov 3, 2025

Addresses #680 (comment)

This PR breaks the existing dependency for openondemand app installs to require a cluster to be deployed in the environment used for image builds.

  • openondemand_*_partition is overridden for builder vars so that ood app install always included in image builds
  • In this case the named partitions don't actually exist and so vars are simply truthy
  • In the common environment, logic handles cases where openhpc_partitions is defined and populated, defined and empty and undefined

sjpb
sjpb requested changes Nov 7, 2025
View reviewed changes
@bertiethorpe bertiethorpe marked this pull request as ready for review November 7, 2025 17:32
@bertiethorpe bertiethorpe requested a review from a team as a code owner November 7, 2025 17:32
@sjpb sjpb self-requested a review November 12, 2025 10:30
sjpb
sjpb reviewed Nov 13, 2025
View reviewed changes
Copy link
Collaborator

@sjpb sjpb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but needs CI running

@sjpb
Copy link
Collaborator

sjpb commented Nov 13, 2025 •
edited
Loading

Image build bumped @ e8c1a39 appears to be https://github.com/stackhpc/ansible-slurm-appliance/actions/runs/19185734179.

@sjpb sjpb self-requested a review November 13, 2025 10:07
sjpb
sjpb requested changes Nov 13, 2025
View reviewed changes
Copy link
Collaborator

@sjpb sjpb left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There also appears to be a later image build run at e306e16 which failed: 601

Is there a reason we don't need that?

@bertiethorpe
Copy link
Member Author

bertiethorpe commented Nov 13, 2025

There also appears to be a later image build run at e306e16 which failed: 601

Is there a reason we don't need that?

That failed build was for SMS, testing purposes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JohnGarbutt JohnGarbutt JohnGarbutt left review comments

@sjpb sjpb sjpb requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

requires-imagebuild

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bertiethorpe @sjpb @JohnGarbutt